Add pull_request_target workflows for community PRs - #1386
Conversation
Adds companion -target.yml workflows to support community PRs that need secrets access, following the established pattern from bitwarden/clients. Problem: Community PRs from forks fail when workflows require secrets (Azure auth, GitHub App tokens, etc.) because fork PRs don't have access to repository or organization secrets when triggered via pull_request events. Solution: Created three new -target.yml workflows that use pull_request_target: - build-wasm-internal-target.yml - build-android-target.yml - enforce-labels-target.yml These workflows: 1. Use check-run.yml to validate the triggering actor has write permissions 2. Only run for external forks (not internal PRs) 3. Call the existing workflows with secrets: inherit 4. Require manual approval - they show as "action_required" and must be re-run by a maintainer after code review Security safeguards: - No direct checkout of PR code (calls reusable workflows) - Fork detection: github.event.pull_request.head.repo.full_name != github.repository - Permission validation via check-run.yml - Manual maintainer approval required before execution Workflow for maintainers: 1. Community PR is opened 2. Workflows fail with "action_required" status 3. Maintainer reviews PR code for safety 4. Maintainer clicks "Re-run jobs" on failed checks 5. Workflows run with secrets access after permission validation This follows the same pattern used in bitwarden/clients for build-cli-target.yml, build-browser-target.yml, etc.
98f9c06 to
2c7c2f9
Compare
🔍 SDK Breaking Change DetectionSDK Version:
Breaking change detection uses the build of the SDK from this branch, including any incompatibities pre-existing on or merged into this branch. Check the workflow logs to confirm. |
- Add workflow_call trigger to build-android.yml and build-wasm-internal.yml to make them reusable workflows - Remove rc and hotfix-rc branch targets from build-wasm-internal-target.yml per linter requirement that pull_request_target can only target main branch
1cbd12e to
8626e44
Compare
🤖 Bitwarden Claude Code ReviewOverall Assessment: REQUEST CHANGES Reviewed three new Code Review Details
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #1386 +/- ##
=======================================
Coverage 86.41% 86.41%
=======================================
Files 501 501
Lines 74235 74235
=======================================
Hits 64149 64149
Misses 10086 10086 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Problem 1: trigger-wasm-publish was firing on all fork PRs - Added event_name check to ensure it only runs on push events - Changed condition from `github.ref == 'refs/heads/main'` to `github.event_name == 'push' && github.ref == 'refs/heads/main'` - This prevents the production npm publish from triggering on every community fork PR Problem 2: combine job had no checkout under pull_request_target - Updated PR checkout step to accept both pull_request and pull_request_target events - Changed ref from head.ref to head.sha for more reliable checkout - Added repository parameter to support fork PRs - This ensures the combine job can actually run when called from build-android-target.yml
9ad22a1 to
fb25ff8
Compare
The pull_request trigger still runs on fork PRs, but fork PRs don't
have access to secrets or write permissions. This causes the following
jobs to fail:
In build-android.yml:
- combine: needs packages:write for gradle publish
- check-android-breaking-changes: needs AZURE_* secrets
In build-wasm-internal.yml:
- trigger-breaking-change-check: needs AZURE_* secrets
Added fork detection to skip these jobs for external PRs:
if: github.event_name != 'pull_request' ||
github.event.pull_request.head.repo.full_name == github.repository
This ensures:
- Internal PRs run all checks normally via pull_request
- Fork PRs skip secrets-dependent jobs in pull_request run
- Fork PRs get secrets-dependent jobs via pull_request_target after
maintainer approval
When called from pull_request_target workflows, the reusable workflow inherits the caller's event context, causing several issues: 1. Checkout had no ref parameter, checked out base branch instead of PR 2. Version setting only ran for pull_request, leaving vars unset 3. Breaking-change check only ran for pull_request, was skipped 4. Integration tests checkout also had no ref parameter Changes: - Split checkout into PR and Push/Dispatch variants like build-android - PR checkout uses head.sha and repository for fork support - Updated version setting to accept pull_request_target - Updated breaking-change check to run for: - Internal PRs via pull_request (has secrets) - Fork PRs via pull_request_target (has secrets after approval) - Skip for fork PRs via pull_request (no secrets) - Fixed integration-tests checkout the same way This ensures fork PRs build the actual PR code and run breaking-change detection after maintainer approval.
coroiu
left a comment
There was a problem hiding this comment.
Looks good, just one question
The base enforce-labels.yml workflow only needs read permissions (contents: read, pull-requests: read) and doesn't require any secrets. Fork PRs can already run it successfully under the pull_request trigger. The -target version added unnecessary security risk by running with pull_request_target privileges for a workflow that only performs read operations.
…target workflows for community PRs (bitwarden/sdk-internal#1386)
Summary
Adds companion
-target.ymlworkflows to support community PRs that need secrets access, following the established pattern frombitwarden/clients.Problem
Community PRs from forks fail when workflows require secrets (Azure auth, GitHub App tokens, etc.) because fork PRs don't have access to repository or organization secrets when triggered via
pull_requestevents.Current failures on community PRs:
How it works
These workflows:
Checks that the user has write permissions with
check-run.yml, only runs on external forks, calls the existing workflows withsecrets: inherit, requires manual approval - they show asaction_requiredand must be re-run by a maintainer after code reviewRelated:
This follows the exact same pattern used in
bitwarden/clients: